Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Apr 24, 2025

Release notes

  • The keystore has improved validation, and will no longer allow the creation of secrets that cannot be accessed with config replacement parameters (${ ... }).

What does this PR do?

Before this change, key-value can be added to keystore but it might impossible to reference the key because ConfigVariableExpander ignores if key name doesn't align with SUBSTITUTION_PLACEHOLDER_REGEX
Improves user experience with the secret store CLI that when providing key name, LS validates to accept a value which can be used in pipeline (aligns with ConfigVariableExpander#SUBSTITUTION_PLACEHOLDER_REGEX).
If already invalid key(s) were added, it warns to remove/replace the keys.

Why is it important/What is the impact to the user?

No user impact. Keystore CLI restricts adding meaningless keys to keystore.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [] I have made corresponding changes to the documentation
  • [] I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs


This is an automatic backport of pull request #17351 done by [Mergify](https://mergify.com).

* Improve the key validation in secret identifier.

* Restrict adding invalid (from ConfigVariableExpander point of view) key names to the keystore through keystore CLI. Keystore now warns on invalid keys if they were already added.

* Key pattern is moved to a central config expander class with its description.

Co-authored-by: Rye Biesemeyer <[email protected]>
(cherry picked from commit d24675a)

# Conflicts:
#	docs/reference/keystore.md
@mergify mergify bot added backport conflicts Detected git conflicts labels Apr 24, 2025
@mergify
Copy link
Contributor Author

mergify bot commented Apr 24, 2025

Cherry-pick of d24675a has failed:

On branch mergify/bp/8.18/pr-17351
Your branch is up to date with 'origin/8.18'.

You are currently cherry-picking commit d24675ae.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   logstash-core/src/main/java/org/logstash/plugins/ConfigVariableExpander.java
	modified:   logstash-core/src/main/java/org/logstash/secret/SecretIdentifier.java
	modified:   logstash-core/src/main/java/org/logstash/secret/cli/SecretStoreCli.java
	modified:   logstash-core/src/test/java/org/logstash/secret/cli/SecretStoreCliTest.java

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   docs/reference/keystore.md

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@github-actions
Copy link
Contributor

📃 DOCS PREVIEWhttps://logstash_bk_17586.docs-preview.app.elstc.co/diff

@elastic-sonarqube
Copy link

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

cc @mashhurs

@mashhurs
Copy link
Contributor

Rendered doc:
image

Copy link
Contributor

@mashhurs mashhurs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@mashhurs mashhurs merged commit 6e0c2dc into 8.18 Apr 24, 2025
8 checks passed
@mashhurs mashhurs deleted the mergify/bp/8.18/pr-17351 branch April 24, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport conflicts Detected git conflicts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants